bitkeeper revision 1.1159.1.319 (41860923CuMAB3frY4t4g-Ls_iqqzg)

Clean up some Xen comments to clarify execution order w.r.t. TLB
flushes.

diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c
index 6e50febc4f95c55a888f2db5fc27288b8e81fedd..2079bf51c6940de4b0142ae6c0011d23dab4226c 100644 (file)
--- a/xen/arch/x86/flushtlb.c
+++ b/xen/arch/x86/flushtlb.c
@@ -41,7 +41,7 @@ void write_cr3(unsigned long cr3)
     t = tlbflush_clock;
     do {
         t1 = t2 = t;
-        /* Clock wrapped: someone else is leading a global TLB shootodiown. */
+        /* Clock wrapped: someone else is leading a global TLB shootdown. */
         if ( unlikely(t1 == 0) )
             goto skip_clocktick;
         t2 = (t + 1) & WRAP_MASK;
@@ -60,7 +60,15 @@ void write_cr3(unsigned long cr3)
     __asm__ __volatile__ ( "mov"__OS" %0, %%cr3" : : "r" (cr3) : "memory" );
 
     /*
-     * STEP 3. Update this CPU's timestamp.
+     * STEP 3. Update this CPU's timestamp. Note that this happens *after*
+     *         flushing the TLB, as otherwise we can race a NEED_FLUSH() test
+     *         on another CPU. (e.g., other CPU sees the updated CPU stamp and
+     *         so does not force a synchronous TLB flush, but the flush in this
+     *         function hasn't yet occurred and so the TLB might be stale).
+     *         The ordering would only actually matter if this function were
+     *         interruptible, and something that abuses the stale mapping could
+     *         exist in an interrupt handler. In fact neither of these is the
+     *         case, so really we are being ultra paranoid.
      */
 
     tlbflush_time[smp_processor_id()] = t2;

diff --git a/xen/arch/x86/memory.c b/xen/arch/x86/memory.c
index 1d390933a189fc094399676fe4f418dccf2f9cd7..410829cc7283c067729ad445c971d2b116081105 100644 (file)
--- a/xen/arch/x86/memory.c
+++ b/xen/arch/x86/memory.c
@@ -1593,7 +1593,7 @@ void ptwr_flush(const int which)
         MEM_LOG("ptwr: Could not read pte at %p\n", ptep);
         /*
          * Really a bug. We could read this PTE during the initial fault,
-         * and pagetables can't have changed meantime. XXX Multi-proc guests?
+         * and pagetables can't have changed meantime. XXX Multi-CPU guests?
          */
         BUG();
     }
@@ -1620,13 +1620,14 @@ void ptwr_flush(const int which)
         MEM_LOG("ptwr: Could not update pte at %p\n", ptep);
         /*
          * Really a bug. We could write this PTE during the initial fault,
-         * and pagetables can't have changed meantime. XXX Multi-proc guests?
+         * and pagetables can't have changed meantime. XXX Multi-CPU guests?
          */
         BUG();
     }
 
     /* Ensure that there are no stale writable mappings in any TLB. */
-    __flush_tlb_one(l1va);
+    /* NB. INVLPG is a serialising instruction: flushes pending updates. */
+    __flush_tlb_one(l1va); /* XXX Multi-CPU guests? */
     PTWR_PRINTK("[%c] disconnected_l1va at %p now %08lx\n",
                 PTWR_PRINT_WHICH, ptep, pte);
 
@@ -1766,7 +1767,7 @@ int ptwr_do_page_fault(unsigned long addr)
     {
         nl2e = mk_l2_pgentry(l2_pgentry_val(*pl2e) & ~_PAGE_PRESENT);
         update_l2e(pl2e, *pl2e, nl2e);
-        flush_tlb();
+        flush_tlb(); /* XXX Multi-CPU guests? */
     }
     
     /* Temporarily map the L1 page, and make a copy of it. */